feat(workspace-studio): Roux Ingest HTTP-mode add-on endpoint - #112

Deploying with    Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

sequenceDiagram
  participant AppsScript
  participant WorkspaceAuth
  participant ExecuteHandler
  participant ChannelRegistry
  participant DB as cc_intents
  participant GoalService as createGoal
  participant PlanService as createPlan
  participant IntentService as createRouxIngestIntentIdempotent
  participant ExecutionCtx as waitUntil

  AppsScript->>WorkspaceAuth: POST /execute with authorizationEventObject
  WorkspaceAuth->>ExecuteHandler: validated workspaceContext + body
  ExecuteHandler->>ChannelRegistry: verifyRegisteredChannel(channel_id)
  ChannelRegistry-->>ExecuteHandler: ChannelMeta | null
  ExecuteHandler->>DB: SELECT by message_id
  alt existing intent
    DB-->>ExecuteHandler: existing intent row
    ExecuteHandler-->>AppsScript: stepSuccess (idempotent_hit: true)
  else no existing
    ExecuteHandler->>GoalService: createGoal(...)
    GoalService-->>ExecuteHandler: goal_id
    ExecuteHandler->>PlanService: createPlan(goal_id,...)
    PlanService-->>ExecuteHandler: plan_id
    ExecuteHandler->>IntentService: createRouxIngestIntentIdempotent(plan_id,...)
    IntentService-->>ExecuteHandler: intent_id, created:true
    ExecuteHandler->>ExecutionCtx: waitUntil(ingestAttachment, recordCustody, classify)
    ExecuteHandler-->>AppsScript: stepSuccess (intent_id)
  end

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

1. @coderabbitai review
2. @copilot review
3. @codex review
4. @claude review
   Adversarial review request: evaluate security, policy bypass paths, regression risk, and merge-gating bypass attempts.

To use Codex here, create a Codex account and connect to github.

1. @coderabbitai review
2. @copilot review
3. @codex review
4. @claude review
   Adversarial review request: evaluate security, policy bypass paths, regression risk, and merge-gating bypass attempts.

To use Codex here, create a Codex account and connect to github.

1. @coderabbitai review
2. @copilot review
3. @codex review
4. @claude review
   Adversarial review request: evaluate security, policy bypass paths, regression risk, and merge-gating bypass attempts.

To use Codex here, create a Codex account and connect to github.

#!/bin/bash
echo "=== src/lib/workspace-jwt.ts ===" && cat src/lib/workspace-jwt.ts
echo ""
echo "=== src/lib/channel-registry.ts ===" && cat src/lib/channel-registry.ts
echo ""
echo "=== src/middleware/workspace-auth.ts ===" && cat src/middleware/workspace-auth.ts
echo ""
echo "=== src/routes/workspace-studio.ts ===" && cat src/routes/workspace-studio.ts
echo ""
echo "=== src/index.ts ===" && cat src/index.ts
echo ""
echo "=== tests/routes/workspace-studio-ingest.spec.ts ===" && cat tests/routes/workspace-studio-ingest.spec.ts

@chittyos — adversarial review below, covering security, policy bypass paths, regression risk, and merge-gate bypass. Issues are ranked by severity.

Adversarial Security Review — PR #112 (Phase 2 Workspace Studio Roux Ingest)

🔴 CRITICAL

1. Algorithm Confusion Attack — verifyWithJWKS (src/lib/workspace-jwt.ts, ~Line 92)

const alg = header.alg || (match.alg as string) || 'RS256';
const key = await importJWK(match, alg);
try {
  const { payload } = await jwtVerify(token, key, {
    audience: expectedAudience,
    algorithms: [alg],
  });

The alg is read from the untrusted JWT header first. This is the classic RS256→HS256 algorithm-confusion attack: an adversary crafts a token with alg: "HS256" in the header; importJWK is called on the RSA public JWK with algorithm HS256; and jwtVerify is restricted to ['HS256']. Whether jose permits this import and verification depends on the JWK's kty — jose v6 will likely throw on importJWK(rsaJwk, 'HS256'), but this path has not been tested, is undocumented behaviour, and must not be relied on.

Even if jose defends today, the pattern is wrong. Algorithm must come from the trusted JWK or be hardcoded; never from the token header.

Fix:

// Always use RS256 — Google Workspace tokens are RS256. Never trust header.alg.
const alg = (match.alg as string) || 'RS256';
const key = await importJWK(match, alg);
const { payload } = await jwtVerify(token, key, {
  audience: expectedAudience,
  algorithms: ['RS256'],   // ← hardcoded, not from header
});

2. JWKS Cache Poisoning via COMMAND_KV (src/lib/workspace-jwt.ts, Lines ~65–82)

The JWKS is cached under the fixed, unprefixed key gcp:jwks in COMMAND_KV — the same namespace used by every other route in this worker. Any other handler, scheduled job, or KV write path that can write to COMMAND_KV can replace gcp:jwks with an attacker-controlled public key. After the next cache miss/expiry is skipped by the TTL check, all subsequent token verifications use the attacker's key.

This is a lateral-movement vector: a lower-privilege code path (e.g., a compromised sync route that writes state to KV) can escalate to authentication bypass on the Workspace endpoint.

Mitigations (pick all three):

1. Namespace the KV key: ws:gcp:jwks and document that it must be treated as security-sensitive.
2. On cache hit, verify the cached keys array against a pinned key count (≥1) and validate that at least one key has alg: RS256 and kty: RSA.
3. When GCP_JWKS_URL points to DEFAULT_JWKS_URL (Google), consider also pinning the HTTPS cert / domain rather than trusting the env var alone.

🟠 HIGH

3. kid-less Token Falls Back to jwks[0] — Attacker Controls Key Selection (src/lib/workspace-jwt.ts, Line ~89)

const match = jwks.find((k) => k.kid === header.kid) ?? jwks[0];

If the JWT header omits kid, or carries a kid that doesn't match any cached key, the code silently falls back to the first JWK in the array. Combined with finding #2 (cache poisoning), an attacker who inserts a poisoned JWKS where their key is first can forge tokens even with any kid — or simply craft tokens without a kid.

Fix: Require kid matching. If no match is found, reject with JWKS_NO_MATCH rather than falling back.

const match = jwks.find((k) => k.kid && k.kid === header.kid);
if (!match) {
  throw new WorkspaceJWTError('JWKS_NO_MATCH', `No JWK matching kid=${header.kid}`);
}

4. channel_id Defaults to Canonical ID — Policy Bypass by Omission (src/routes/workspace-studio.ts, Lines ~77–81)

const channelId =
  extractScalar(body, 'channel_id') ??
  extractInputScalar(body, 'channel_id') ??
  WORKSPACE_STUDIO_CHANNEL_ID;         // ← silently defaults

Any request bearer with a valid Google JWT (correctly signed systemIdToken + userIdToken) that omits channel_id automatically gets matched to the registered canonical channel, completely bypassing the channel registration gate. The channel check then passes, including the capability gate.

This means the channel verification provides no additional security boundary over JWT auth alone. A third party that obtains valid tokens (e.g. via a leaked service account credential or a phished user) and knows this endpoint can omit channel_id and proceed.

Fix: Remove the default; make channel_id a required field.

const channelId = extractScalar(body, 'channel_id') ?? extractInputScalar(body, 'channel_id');
if (!channelId) {
  return c.json(stepError('MISSING_INPUT', 'channel_id is required', 'NOT_RETRYABLE'), 400);
}

5. verifyRegisteredChannel Called Twice — Future TOCTOU (v2) (src/routes/workspace-studio.ts, Lines ~83–100)

const channelExists = await verifyRegisteredChannel(channelId, c.env);          // call 1
if (!channelExists) { … 403 … }
const channel = await verifyRegisteredChannel(channelId, c.env, REQUIRED_CAPS); // call 2
if (!channel) { … 403 … }

In v1 (env-var) these two calls are idempotent. In the planned v2 (HTTP fetch to agent.chitty.cc), two sequential network calls to the channel registry introduce a real TOCTOU window: a channel could be suspended between call 1 and call 2, or more dangerously, a cache entry could be modified in the KV store between calls.

Fix now, before v2 ships: Collapse into one call with capabilities required, and infer the error case:

const channel = await verifyRegisteredChannel(channelId, c.env, REQUIRED_CAPS);
if (!channel) {
  // Could be not-registered, suspended, or missing capability — surface all as 403
  return c.json(stepError('CHANNEL_REJECTED', `Channel ${channelId} is not authorized`, 'NOT_RETRYABLE'), 403);
}

If the error distinction between NOT_REGISTERED and MISSING_CAPABILITY is operationally required, add a separate channelExistsNoCapCheck flag to the registry return type instead of making two async calls.

🟡 MEDIUM

6. /config Endpoint Has No Authentication

workspaceStudioRoutes.post('/config', async (c) => {
  // Config endpoint does not require user auth
  return c.json({ sections: [ … ] });
});

While the config card is currently static JSON, it reveals internal service names, endpoint URLs (https://command.chitty.cc), default values, and implementation details to any unauthenticated caller. There is also no CORS restriction on this sub-route. As the config endpoint evolves (e.g. if it starts reading channel state or user preferences), the absence of auth becomes a data-exposure risk.

Recommendation: Apply workspaceAuth() to /config as well, consistent with the execute endpoint.

7. Idempotency-Key Header Is Attacker-Controlled and Stored in Intent Metadata

const idempotencyKey = c.req.header('Idempotency-Key') ?? `gmail-${messageId}`;

This header is stored in intent.metadata.idempotency_key and surfaced in the success response. A caller can inject arbitrary values (log injection characters, SQLi-looking strings, PII). The field is not validated or sanitized. While not currently used for deduplication logic, if future code uses idempotency_key for lookups it could be exploited.

Fix: Validate or discard the header — if you accept it, constrain to [a-zA-Z0-9_-]{1,128}:

const raw = c.req.header('Idempotency-Key');
const idempotencyKey = (raw && /^[a-zA-Z0-9_\-]{1,128}$/.test(raw)) ? raw : `gmail-${messageId}`;

8. Real GCP Identifiers Committed in Test File (tests/routes/workspace-studio-ingest.spec.ts, Lines ~35–36)

const SA_EMAIL = 'chittyclaw@chittyops.iam.gserviceaccount.com';
const CLIENT_ID = '443939537625-a0un9jpol6gi53h7t53kbn4jic0o3c0m.apps.googleusercontent.com';

The GCP project number (443939537625) and the service account name (chittyclaw@chittyops) are now permanently in git history. Even if rotated later, this exposes the GCP project topology to anyone with repo read access. The OAuth client ID is especially sensitive — it is a stable identifier usable in phishing or scope-escalation attacks against users of this add-on.

Fix: Move to environment variables read from the test environment, never hardcoded.

9. content_hashes Is Always Empty — Fanout Never Updates Intent Payload

payload: {
  …
  content_hashes: [] as string[],   // ← never populated
},

The ingestAttachment fanout fires asynchronously but never writes back to the intent's content_hashes field. The success response also returns content_hashes: []. This means the custody chain has no content hash anchors for Gmail attachments — breaking the evidentiary integrity guarantee for legalink/privileged content, which is the exact use-case this endpoint is designed to serve.

This is a data integrity regression against the custody/evidence requirements described in the PR objectives.

🟢 LOW / INFORMATIONAL

10. DB-Backed Integration Tests Always Skipped in CI — De-facto Merge-Gate Bypass

describe.skipIf(SKIP_DB)('workspace-studio route (real Neon)', () => { … });

Without a DATABASE_URL secret provisioned in CI, 5 of 9 tests never run. The PR can be merged with broken intent creation, broken idempotency, broken Roux derivation, or broken channel enforcement — none would be caught. The 4 passing tests are JWT-only (no DB).

Recommendation: Either provision a Neon branch in CI (as planned), or add a CI step that fails if SKIP_DB=1 and any skipIf tests are detected. At minimum, the PR description should note this as a blocking follow-up.

11. CHITTYROUX_MARKETPLACE_OAUTH_CLIENT_SECRET Declared but Never Referenced

CHITTYROUX_MARKETPLACE_OAUTH_CLIENT_SECRET?: string;  // in Env

This secret is declared in Env but never used anywhere in the implementation. If it's intended for future OAuth flows, document it. If it was supposed to be used for request signing or client credential validation, its absence is a missing security control.

12. User OAuth Token Forwarded in Plaintext JSON Body to Internal Storage

body: JSON.stringify({
  source: 'gmail',
  attachment_id: attachmentId,
  user_oauth_token: userOAuthToken,   // ← live OAuth token in request body
  intent_id: intentId,
}),

The live user_oauth_token (a Google OAuth bearer) is serialized into a JSON request body to SVC_STORAGE. Any logging, tracing, or error capture in the storage service that logs request bodies will capture a live OAuth token. Prefer passing it in a request header (Authorization: Bearer …) which is more likely to be redacted by logging middleware.

Summary Table

Issues #1, #2, #3, and #4 should be resolved before merge. #9 is a functional regression against the PR's own stated evidentiary integrity requirements and should also block merge.

1. @coderabbitai review
2. @copilot review
3. @codex review
4. @claude review
   Adversarial review request: evaluate security, policy bypass paths, regression risk, and merge-gating bypass attempts.

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

To use Codex here, create a Codex account and connect to github.

My adversarial security review for this PR is already posted above in this thread. Here's a quick summary for the other reviewers:

Merge-blocking issues (resolve before merge):

Additional findings (non-blocking but should be tracked): double verifyRegisteredChannel call (TOCTOU in v2), unauthenticated /config endpoint, unsanitized Idempotency-Key header, real GCP SA email + OAuth client ID hardcoded in tests, DB-backed tests always skipped (merge-gate bypass), unused CLIENT_SECRET env var, OAuth token in request body instead of Authorization header.

Triggering a full review pass now.

Learnt from: CR
Repo: chittyos/chittycommand PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-06T22:38:24.770Z
Learning: Applies to src/middleware/auth.ts : The three auth middlewares (`authMiddleware`, `bridgeAuthMiddleware`, `mcpAuthMiddleware`) must not be bypassed or modified without security review

Learnt from: github-actions[bot]
Repo: chittyos/chittycommand PR: 0
File: :0-0
Timestamp: 2026-03-11T12:29:34.011Z
Learning: In chittyos/chittycommand src/middleware/auth.ts: The bridge service token comparison uses non-timing-safe string equality `token === serviceToken` instead of a constant-time compare function, creating a timing oracle for the bridge:service_token KV value.